Skip to content

Doc fixes for core::future::Future. #60128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 20, 2019
Merged

Conversation

jimblandy
Copy link
Contributor

Fixed outdated reference to waker argument; now futures are passed a
Context, from which one can obtain a waker.

Cleaned up explanation of what happens when you call poll on a completed
future. It doesn't make sense to say that poll implementations can't cause
memory unsafety; no safe function is ever allowed to cause memory unsafety, so
why mention it here? It seems like the intent is to say that the Future trait
doesn't say what the consequences of excess polls will be, and they might be
bad; but that the usual constraints that Rust imposes on any non-unsafe
function still apply. It's also oddly specific to say 'memory corruption'
instead of just 'undefined behavior'; UB is a bit jargony, so the text should
provide examples.

Fixed outdated reference to `waker` argument; now futures are passed a
`Context`, from which one can obtain a `waker`.

Cleaned up explanation of what happens when you call `poll` on a completed
future. It doesn't make sense to say that `poll` implementations can't cause
memory unsafety; no safe function is ever allowed to cause memory unsafety, so
why mention it here? It seems like the intent is to say that the `Future` trait
doesn't say what the consequences of excess polls will be, and they might be
bad; but that the usual constraints that Rust imposes on any non-`unsafe`
function still apply. It's also oddly specific to say 'memory corruption'
instead of just 'undefined behavior'; UB is a bit jargony, so the text should
provide examples.
@rust-highfive
Copy link
Contributor

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 20, 2019
@sfackler
Copy link
Member

r? @withoutboats

@withoutboats
Copy link
Contributor

looks like a clear improvement, thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 20, 2019

📌 Commit f8f02de has been approved by withoutboats

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 20, 2019
bors added a commit that referenced this pull request Apr 20, 2019
Doc fixes for core::future::Future.

Fixed outdated reference to `waker` argument; now futures are passed a
`Context`, from which one can obtain a `waker`.

Cleaned up explanation of what happens when you call `poll` on a completed
future. It doesn't make sense to say that `poll` implementations can't cause
memory unsafety; no safe function is ever allowed to cause memory unsafety, so
why mention it here? It seems like the intent is to say that the `Future` trait
doesn't say what the consequences of excess polls will be, and they might be
bad; but that the usual constraints that Rust imposes on any non-`unsafe`
function still apply. It's also oddly specific to say 'memory corruption'
instead of just 'undefined behavior'; UB is a bit jargony, so the text should
provide examples.
@bors
Copy link
Collaborator

bors commented Apr 20, 2019

⌛ Testing commit f8f02de with merge 647a951...

@bors
Copy link
Collaborator

bors commented Apr 20, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: withoutboats
Pushing 647a951 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 20, 2019
@bors bors merged commit f8f02de into rust-lang:master Apr 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants